Skip to content

Prevent HTTP handler goroutines from blocking forever (F-ssv-dkg-007)#229

Merged
julienh-ssv merged 7 commits intomainfrom
F-ssv-dkg-007
Apr 13, 2026
Merged

Prevent HTTP handler goroutines from blocking forever (F-ssv-dkg-007)#229
julienh-ssv merged 7 commits intomainfrom
F-ssv-dkg-007

Conversation

@julienh-ssv
Copy link
Copy Markdown
Contributor

Fix for:

Blocking Channel Reads with No Timeout
ssv-dkg · Security · Assigned: dev4 · Effort: —
ProcessMessages blocks indefinitely on <-iw.respChan. CreateInstance blocks on <-bchan. If the DKG protocol stalls (lost message, deadlock), the HTTP handler goroutine is permanently blocked.

Impact: Attacker can exhaust all server goroutines by triggering protocol stalls, making the operator unresponsive to all requests.

Recommendation: Use select with context.WithTimeout for all channel reads. A reasonable timeout is the MaxInstanceTime (currently 1 minute).

@julienh-ssv julienh-ssv requested a review from olegshmuelov April 1, 2026 09:56
@julienh-ssv julienh-ssv self-assigned this Apr 1, 2026
Copy link
Copy Markdown
Contributor

@olegshmuelov olegshmuelov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout covers channel reads in ProcessMessages and CreateInstance, but there is another blocking point in the same HTTP handler path: processDKG() writes to unbuffered board channels (o.board.DealC <- *b, ResponseC, JustificationC). If the DKG protocol goroutine has exited or is stuck, this write blocks forever before the select timeout fires — same attack vector.

Worth addressing in this PR since it is the same code path and the same finding. A buffered channel (capacity 1) or a select with timeout on the write side would close the gap.

Separately, the DKG protocol goroutines (StartDKG, StartReshareDKG*) that wait on p.WaitEnd() will also leak if the protocol stalls — not a handler-blocking issue but a resource leak. Worth a separate follow-up finding.

Comment thread pkgs/operator/state.go
Comment thread pkgs/operator/instance.go Outdated
@julienh-ssv julienh-ssv requested a review from olegshmuelov April 5, 2026 04:53
Comment thread pkgs/operator/state.go Outdated
@julienh-ssv julienh-ssv requested a review from olegshmuelov April 5, 2026 14:19
olegshmuelov
olegshmuelov previously approved these changes Apr 5, 2026
Copy link
Copy Markdown
Contributor

@olegshmuelov olegshmuelov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All concerns addressed, board channel write timeouts, identity-guarded cleanup, reflect simplification. Clean implementation.

One follow-up worth tracking separately: the DKG protocol goroutines (StartDKG, StartReshareDKG*) that wait on p.WaitEnd() will leak if the protocol stalls. Not a handler-blocking issue, but a resource leak under the same attack model. Can you open a follow-up for that in Github and the board pls?

olegshmuelov
olegshmuelov previously approved these changes Apr 7, 2026
Copy link
Copy Markdown
Contributor

@olegshmuelov olegshmuelov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread pkgs/operator/state.go Outdated
nkryuchkov
nkryuchkov previously approved these changes Apr 10, 2026
@julienh-ssv julienh-ssv changed the title Prevent HTTP handler goroutines from blocking forever Prevent HTTP handler goroutines from blocking forever (F-ssv-dkg-007) Apr 12, 2026
Copy link
Copy Markdown
Contributor

@olegshmuelov olegshmuelov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@julienh-ssv julienh-ssv merged commit d7eb2bd into main Apr 13, 2026
5 checks passed
@julienh-ssv julienh-ssv deleted the F-ssv-dkg-007 branch April 13, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants